Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update and clean up JSDocs #38 #39

Closed
wants to merge 1 commit into from
Closed

Conversation

salilbc
Copy link

@salilbc salilbc commented Oct 28, 2022

No description provided.

Signed-off-by: Salil Cuncoliencar <Salil_Cuncoliencar@intuit.com>
Copy link
Collaborator

@nsinghal12 nsinghal12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please document the TS types and other constants/methods as defined and used. Also, fix the PR title to being with Fix #38 and add proper PR description on what this PR does.

* @param {object} opt Is the option with the filePath or xmlContent and the optional format.
* @return {object} The pom object along with the timers.
* @param {*} opt Is the option with the filePath or xmlContent and the optional format.
* @param {*} callback The pom object along with the timers.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix up the leading text to param, and also use TS type.

@@ -23,8 +23,8 @@ type ParseCallback=(e: Error | null, r?: ParsedOutput | null) => void;

/**
* Parses xml into javascript object by using a file path or an xml content.
* @param {object} opt Is the option with the filePath or xmlContent and the optional format.
* @return {object} The pom object along with the timers.
* @param {*} opt Is the option with the filePath or xmlContent and the optional format.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of * use the TS type here.

@@ -103,6 +101,12 @@ function removeSingleArrays(obj: Object):void {
});
}

/**
* Reads a file in Async mode
* @param {*} path is the file path
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change type to string.

/**
* Reads a file in Async mode
* @param {*} path is the file path
* @param {*} encoding is the type on encoding used in the file
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use correct type here.

* Reads a file in Async mode
* @param {*} path is the file path
* @param {*} encoding is the type on encoding used in the file
* @returns
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing returns.

@@ -65,8 +65,6 @@ function parse(opt: ParseOptions,callback: ParseCallback): void {
/**
* Parses the given xml content.
* @param xmlContent {string} Is the xml content in string using utf-8 format.
* @param loadedXml {boolean} Whether the xml was loaded from the file-system.
* @param callback {function} The callback function using Javascript PCS.
*/
function _parseWithXml2js(xmlContent: string): Promise<ParsedOutput> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing return doc.

@nsinghal12
Copy link
Collaborator

Closing stale PR.

@nsinghal12 nsinghal12 closed this Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants